Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #9261 - Use decimal symbol configured in system and user #10495

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Fix #9261 - Use decimal symbol configured in system and user #10495

merged 1 commit into from
Oct 15, 2024

Conversation

SinergiaCRM
Copy link
Contributor

@SinergiaCRM SinergiaCRM commented Aug 7, 2024 •
edited
Loading

Description

This PR fixes the decimal separator symbol for a decimal or float value obtained from a database not being parsed correctly when incorporated into an email template or PDF template.

To do this, a generic function is created in the PR with a parameter where you can indicate which configuration to use: the system configuration (configured by default) or the user configuration. This function will be used in the operation of parsing the PDF template or the email template.

Our proposal is to use the system configuration for sending emails and the user configuration for using the PDF template.

Motivation and Context

DecimalSymbolParser

How To Test This

  1. Create a decimal and a float fields in a contact module

  2. Set a symbol to separate decimals in user profile and system configuration.

  3. Create a contact-based email template that contains the decimal and float fields.

  4. Create a campaign and use the previous template.

  5. Send a email and check that in the sent email, the decimal and float values is being parsed with the configured system decimal symbol

  6. Create a workflow based on Contacts and edit a Contact that has a value in the decimal and float fields for the workflow to send the email with the previous template

  7. Check that in the sent email, the decimal and float value is being parsed with the configured system decimal symbol

  8. Create a PDF template based on Contact and include the decimal and float fields

  9. Generate a PDF file of a contact that has a value in the decimal field and
    check that the decimal symbol is applied in the generated PDF template

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

@SuiteBot
Copy link

SuiteBot commented Aug 7, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@serhiisamko091184
Copy link
Contributor

Hello @SinergiaCRM!

Thanks for so many contributions in the recent days!

Could you, please, remove comments in templateParser.php file:

image

Thanks in advance!

Regards,
Serhii

@serhiisamko091184 serhiisamko091184 added the Status:Requires Updates Issues & PRs which requires input or update from the author label Aug 8, 2024
@SinergiaCRM
Copy link
Contributor Author

Hello @serhiisamko091184, comments deleted 👍

Regards!!

@serhiisamko091184 serhiisamko091184 added Status: Requires Code Review Needs the core team to code review Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Branch:Hotfix PR 4-8 Score given to PRs once assessed and removed Status:Requires Updates Issues & PRs which requires input or update from the author labels Aug 8, 2024
@serhiisamko091184
Copy link
Contributor

@SinergiaCRM, many thanks for the quick reply and the changes made!

Regards,
Serhii

@SinergiaCRM
Copy link
Contributor Author

Hi, we have detected that it also occurs with float type fields so we have added the appropriate changes in the PR

@ManuSinergiaCRM ManuSinergiaCRM mentioned this pull request Aug 14, 2024
Merged
@jack7anderson7 jack7anderson7 added Status: Requires Testing Requires Manual Testing Status: Passed Code Review Mark issue has passed code review reviewed and removed Status: Requires Code Review Needs the core team to code review labels Aug 22, 2024
johnM2401
johnM2401 approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@johnM2401 johnM2401 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
Now using System-set values on Emails and User-set values on PDFs.

I have noticed that this issue is also present for currency-type fields,
see this email:
image

For the above, I have set:
System Thousand: "X" + System Decimal: "Z"
User Thousand: "." + User Decimal: ","

And the custom Currency->Amount Field is using the user's settings.

However, as this PR resolves the issue for both Decimal and Float, I'm happy to approve as-is.

Thank you again!

@johnM2401 johnM2401 added Status: Passed Testing and removed Status: Requires Testing Requires Manual Testing labels Sep 9, 2024
@jack7anderson7 jack7anderson7 merged commit 2395b2a into salesagility:hotfix Oct 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jack7anderson7 jack7anderson7 jack7anderson7 approved these changes

@johnM2401 johnM2401 johnM2401 approved these changes

Assignees
No one assigned
Labels
Branch:Hotfix PR 4-8 Score given to PRs once assessed Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status: Passed Code Review Mark issue has passed code review reviewed Status: Passed Testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SinergiaCRM @SuiteBot @serhiisamko091184 @johnM2401 @jack7anderson7